Skip to content

python3.12 upgrade#116

Merged
Rob-Johnson merged 6 commits intomasterfrom
u/robj/py312-upgrade
Mar 24, 2026
Merged

python3.12 upgrade#116
Rob-Johnson merged 6 commits intomasterfrom
u/robj/py312-upgrade

Conversation

@Rob-Johnson
Copy link
Contributor

@Rob-Johnson Rob-Johnson commented Mar 16, 2026

we should just vendor this into paasta-tools, but for now, I'm doing the upgrade. no pre-commit here, so no big diff - I'll come back and add that in another PR

precursor for python3.12
@Rob-Johnson Rob-Johnson force-pushed the u/robj/py312-upgrade branch from 296a7da to 383fe25 Compare March 16, 2026 16:29
python3.7-dev \
python3.7-distutils \
python3-pip \
libffi-dev \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if only we could ensure that public pypi had wheels for everything - when we could get rid of all the -dev packages :p

Comment on lines +3 to +4
ARG DEBIAN_FRONTEND=noninteractive
ENV DEBIAN_FRONTEND=${DEBIAN_FRONTEND}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

imo, we can just hardcode this :p

"host": MY_IP_ADDRESS,
"port": 1024,
"weight": CPUS,
"weight": 10,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if we're not using CPUS anymore, we probably wanna remove it at the top of the file


echo "installing hacheck"
GIT_SSH_COMMAND="ssh -o UserKnownHostsFile=/dev/null -o StrictHostKeyChecking=no" git clone git@github.yelpcorp.com:packages/hacheck && cd /hacheck && python3.7 -m pip install . && cp /usr/local/bin/ha* /usr/bin/
GIT_SSH_COMMAND="ssh -o UserKnownHostsFile=/dev/null -o StrictHostKeyChecking=no" git clone --depth=1 git@github.yelpcorp.com:packages/hacheck /hacheck
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

heh, i guess we haven't been expecting folks to run these itests outside of yelp :p

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, not sure what else we can do now we've taken hacheck internal

Comment on lines +39 to +40
ADD dockerfiles/jammy/location_types.json /nail/etc/services/
ADD dockerfiles/jammy/location_mapping.json /nail/etc/services/
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is this because of context: ../.. in the docker-compose file?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah - just copied how we do it elsewhere

Comment on lines +2 to +3
Section: admin
Priority: optional
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we just leave these out? i was a little confused as to whether this was correct since this is duplicated below (but then i realized that there's two different "sections" here)

i don't think we have anything that really cares about these being set unless they're required by the debian/compat bump - which i also don't know much about what that implies/entails

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, excluding them will trigger a warning.

Source: nerve-tools
Section: admin
Priority: optional
Maintainer: John Billings <billings@yelp.com>
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

!8ball clean this up while we're here

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can do - I was just going to leave it for the sake of a smaller diff

Comment on lines +13 to +18
def get_install_requires():
return [
line.strip()
for line in (HERE / 'requirements.txt').read_text().splitlines()
if line.strip() and not line.startswith('#')
]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

heh, we haven't really been doing the right thing with install_requires here since the beginning - but that's for later us to fix (maybe) :p

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, I can break this out into a separate PR

bumps dependencies, including paasta.
@Rob-Johnson Rob-Johnson force-pushed the u/robj/py312-upgrade branch from 383fe25 to 5067b61 Compare March 23, 2026 12:23
itests depend on internal hacheck so won't be able to run on public
github runners
we didn't previously run the tests in a public environment, so never ran
into this.
@Rob-Johnson
Copy link
Contributor Author

@nemacysts @ilkinmammadzada I've had to adjust the github actions setup here a little. now that hacheck is internal only, we can no longer run the itests (which run a full setup with hacheck included) - we'll have to run that in internal ci, and just run mypy + unit tests here

@Rob-Johnson Rob-Johnson changed the title U/robj/py312 upgrade python3.12 upgrade Mar 23, 2026
@Rob-Johnson Rob-Johnson merged commit 1a65071 into master Mar 24, 2026
2 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants